is it worth adding this check for hexadecimal here? we have to convert it anyways

Very good question. Actually our decoding error message is far superior than the one from the validate package. For now we can leave it, but we should reevaluate if we even want to use validate since we need to decode anyway.

typo: Exeuction

The file name is too generic. What's the difference between structs.go and api_structs.go? I would name this one structs_block.go to let know blocks are defined here.

you can use http2.HandleError here and everywhere else to make the code smaller

It's better to write this instead:

slot, valid = shared.ValidateUint(w, "Slot", rawSlot)
if !valid {
	return
}

I would use 503 Status Unavailable here

You can set these headers immediately after you receive the response from the v1alpha1 server. This will reduce duplication and you will not need to pass isBlinded anymore.

i set it but it's still used in the response, moved it outside

this can be inside a conditional

if v1alpha1resp.IsBlinded {
	// check blinded
} else {
	// check unblinded
}

same for capella and deneb

what do you mean by this shouldn't it match the type check?

What I mean is that it doesn't make sense to type check for a blinded block when v1alpha1resp.IsBlinded == false and vice versa.

I would not validate the response. You normally only validate requests. Also this code is incorrect because nothing will happen and 200 will be returned when error is not nil.

what does this comment mean?

I was thinking since it's local build it's not available 🤔 but maybe i'm wrong in that assumption

As stated in another place, I would not add validate to responses nor would I validate them. A validation error will return 400 Bad Request, but it's not the user's request that was incorrect.

I think valueInGwei should either be inside the ExecutionData or BeaconBlock. ExecutionData is more appropriate but I'd hate to make you do this in this PR. Feel free to do this in another PR or open an issue for someone else to do it

let me think about that see if it'd be better to move it or clean this one up

Revert this line?

Better to move this before the return nil after the else

There's no local payload value for Bellatrix, I'd just remove these. Will love to remove any complexity

Same here. We can remove these

You should pass ctx inside. Also, can you add a new line after defer span.End()?

Ok that's true... In that case maybe let's return bytes from shared.ValidateHex. What do you think? And inside the function use hexutil.Decode instead of bytesutil.IsHex

Is this useful? Same comment for other spans in private functions

hmm maybe not maybe I should just pass it down

What I mean is that it doesn't make sense to type check for a blinded block when v1alpha1resp.IsBlinded == false and vice versa.

use ctx

I would rename it to DecodeHexWithLength. Otherwise the caller will have to go into the function to understand it.

Similarly, I would rename to DecodeHexWithMaxLength

Error message is incomplete. But do we even have to validate this once ok passes?

oh weird ok not sure how that cut off will fix

A few comments on this file:

• It would be nice to use NewDecodeError where possible. As a small nitpick, in other conversion code we do not add the name of the object to the error message. So instead of writing NewDecodeError(err, phase0SignedBeaconBlock.Signature) we write NewDecodeError(err, Signature). This is mostly because a user does not send anything called phase0SignedBeaconBlock in the request.
• In https://github.com/prysmaticlabs/prysm/pull/12735/files#diff-a545f9cb2d7ef7f156c12cbef6fd7dc46f45f9e0000681fd79d59c35c38177c9 I used a naming scheme XXXFromConsensus - can you use the same instead of ConvertXXX?
• I don't particularly like the word Internal in exported function names. From my experience "internal" often means that the function is internal to a module. Maybe we could change this to XXXFromConsensus and XXXToConsensus. In case of exits that would become ExitsFromConsensus and ExitsToConsensus.